bitkeeper revision 1.1159.258.90 (425a82f9SNVZLPiwpO07IDSTr_VE7g)

[PATCH]  stream fixes for migration

I've attached a patch for libxutil/libxc.  This fixes one of the hangs =
I've seen during migrations.  It applies against 2.0 and 2.0-testing.

Changes:
 * Encountering EOF or error when xfrd reads from stream could cause an =
infinite loop.
 * Cleaned up the closing of streams.
 * Fixed several memory leaks.

Signed-off-by: Charles Coffing <ccoffing@novell.com>

diff --git a/BitKeeper/etc/logging_ok b/BitKeeper/etc/logging_ok
index dd518ad6c98c0646a14a34ecfa88d184254faa95..470088902542c896678699e3adcf833e8aa74ed2 100644 (file)
--- a/BitKeeper/etc/logging_ok
+++ b/BitKeeper/etc/logging_ok
@@ -10,6 +10,7 @@ br260@br260.wolfson.cam.ac.uk
 br260@labyrinth.cl.cam.ac.uk
 br260@laudney.cl.cam.ac.uk
 bren@anvil.research
+ccoffing@novell.com
 cl349@arcadians.cl.cam.ac.uk
 cl349@firebug.cl.cam.ac.uk
 cl349@freefall.cl.cam.ac.uk

diff --git a/tools/libxc/xc_io.h b/tools/libxc/xc_io.h
index 341f29be6077e2d31a6a5e08cf4308e8f8c668ec..43254735184e47ac234f7fe6a6f9053ff190640e 100644 (file)
--- a/tools/libxc/xc_io.h
+++ b/tools/libxc/xc_io.h
@@ -45,14 +45,14 @@ static inline int xcio_read(XcIOContext *ctxt, void *buf, int n){
     int rc;
 
     rc = IOStream_read(ctxt->io, buf, n);
-    return (rc == n ? 0 : rc);
+    return (rc == n ? 0 : -1);
 }
 
 static inline int xcio_write(XcIOContext *ctxt, void *buf, int n){
     int rc;
 
     rc = IOStream_write(ctxt->io, buf, n);
-    return (rc == n ? 0 : rc);
+    return (rc == n ? 0 : -1);
 }
 
 static inline int xcio_flush(XcIOContext *ctxt){

diff --git a/tools/libxc/xc_linux_save.c b/tools/libxc/xc_linux_save.c
index 6d877c84f0e57c84e05e773f955bea4111fd9405..ab952230dd6a8e449b11674e7a66557be90dbdd8 100644 (file)
--- a/tools/libxc/xc_linux_save.c
+++ b/tools/libxc/xc_linux_save.c
@@ -172,7 +172,6 @@ static int xcio_ratewrite(XcIOContext *ioctxt, void *buf, int n){
     struct timeval now;
     struct timespec delay;
     long long delta;
-    int rc;
 
     if (START_MBIT_RATE == 0)
        return xcio_write(ioctxt, buf, n);
@@ -212,8 +211,7 @@ static int xcio_ratewrite(XcIOContext *ioctxt, void *buf, int n){
            }
        }
     }
-    rc = IOStream_write(ioctxt->io, buf, n);
-    return (rc == n ? 0 : rc);
+    return xcio_write(ioctxt, buf, n);
 }
 
 static int print_stats( int xc_handle, u32 domid, 

diff --git a/tools/libxutil/file_stream.c b/tools/libxutil/file_stream.c
index b5526d84a1485df7e39f4c5d93e67886edd9dd13..ca7b9d16802c8aee95c34cb2ab7aacf67b707b2b 100644 (file)
--- a/tools/libxutil/file_stream.c
+++ b/tools/libxutil/file_stream.c
@@ -151,7 +151,12 @@ static int file_error(IOStream *s){
  * @return result of the close
  */
 static int file_close(IOStream *s){
-    return fclose(get_file(s));
+    int result = 0;
+    if (s->data){
+        result = fclose(get_file(s));
+        s->data = (void*)0;
+    }
+    return result;
 }
 
 /** Free a file stream.
@@ -159,7 +164,7 @@ static int file_close(IOStream *s){
  * @param s file stream
  */
 static void file_free(IOStream *s){
-    // Do nothing - fclose does it all?
+    file_close(s);
 }
 
 /** Create an IOStream for a stream.
@@ -189,7 +194,6 @@ IOStream *file_stream_fopen(const char *file, const char *flags){
        io = file_stream_new(fin);
        if(!io){
            fclose(fin);
-           //free(fin); // fclose frees ?
        }
     }
     return io;
@@ -199,13 +203,16 @@ IOStream *file_stream_fopen(const char *file, const char *flags){
  *
  * @param fd file descriptor
  * @param flags giving the mode to open in (as for fdopen())
- * @return new stream for the open file, or 0 if failed
+ * @return new stream for the open file, or 0 if failed.  Always takes
+ *         ownership of fd.
  */
 IOStream *file_stream_fdopen(int fd, const char *flags){
     IOStream *io = 0;
     FILE *fin = fdopen(fd, flags);
     if(fin){
-       io = file_stream_new(fin);
+        io = file_stream_new(fin);
+        if(!io)
+            fclose(fin);
     }
     return io;
 }

diff --git a/tools/libxutil/gzip_stream.c b/tools/libxutil/gzip_stream.c
index a933c5ff734dc8f5b106ca222e915635c338b015..d3bedbf2175f415bfe6b7a5d1c00cf76f6359467 100644 (file)
--- a/tools/libxutil/gzip_stream.c
+++ b/tools/libxutil/gzip_stream.c
@@ -107,7 +107,12 @@ static int gzip_error(IOStream *s){
  * @return result of the close
  */
 static int gzip_close(IOStream *s){
-    return gzclose(get_gzfile(s));
+    int result = 0;
+    if (s->data){
+        result = gzclose(get_gzfile(s));
+        s->data = (void*)0;
+    }
+    return result;
 }
 
 /** Free a gzip stream.
@@ -115,7 +120,7 @@ static int gzip_close(IOStream *s){
  * @param s gzip stream
  */
 static void gzip_free(IOStream *s){
-    // Do nothing - fclose does it all?
+    gzip_close(s);
 }
 
 /** Create an IOStream for a gzip stream.
@@ -143,11 +148,10 @@ IOStream *gzip_stream_fopen(const char *file, const char *flags){
     gzFile *fgz;
     fgz = gzopen(file, flags);
     if(fgz){
-       io = gzip_stream_new(fgz);
-       if(!io){
-           gzclose(fgz);
-           //free(fgz); // gzclose frees ?
-       }
+        io = gzip_stream_new(fgz);
+        if(!io){
+            gzclose(fgz);
+        }
     }
     return io;
 }
@@ -156,14 +160,17 @@ IOStream *gzip_stream_fopen(const char *file, const char *flags){
  *
  * @param fd file descriptor
  * @param flags giving the mode to open in (as for fdopen())
- * @return new stream for the open file, or NULL if failed
+ * @return new stream for the open file, or NULL if failed.  Always takes
+ *         ownership of fd.
  */
 IOStream *gzip_stream_fdopen(int fd, const char *flags){
     IOStream *io = NULL;
     gzFile *fgz;
     fgz = gzdopen(fd, flags);
     if(fgz){
-       io = gzip_stream_new(fgz);
+        io = gzip_stream_new(fgz);
+        if(!io)
+            gzclose(fgz);
     }
     return io;
 }

diff --git a/tools/libxutil/iostream.h b/tools/libxutil/iostream.h
index 4786f325b29956f95daff7ca119488b010233ec8..1efd8f998527503c4f0c49a2cd45193123720b3a 100644 (file)
--- a/tools/libxutil/iostream.h
+++ b/tools/libxutil/iostream.h
@@ -105,8 +105,11 @@ extern int IOStream_vprint(IOStream *io, const char *format, va_list args);
  * @return if ok, number of bytes read, otherwise negative error code
  */
 static inline int IOStream_read(IOStream *stream, void *buf, size_t n){
-    int result = 0;
-    if(stream->closed) goto exit;
+    int result;
+    if(stream->closed){
+        result = IOSTREAM_EOF;
+        goto exit;
+    }
     if(!stream->methods || !stream->methods->read){
         result = -EINVAL;
         goto exit;
@@ -124,11 +127,14 @@ static inline int IOStream_read(IOStream *stream, void *buf, size_t n){
  * @param stream input
  * @param buf where to put input
  * @param n number of bytes to write
- * @return if ok, number of bytes read, otherwise negative error code
+ * @return if ok, number of bytes written, otherwise negative error code
  */
 static inline int IOStream_write(IOStream *stream, const void *buf, size_t n){
-    int result = 0;
-    if(stream->closed) goto exit;
+    int result;
+    if(stream->closed){
+        result = IOSTREAM_EOF;
+        goto exit;
+    }
     if(!stream->methods || !stream->methods->write){
         result = -EINVAL;
         goto exit;
@@ -179,6 +185,7 @@ static inline int IOStream_close(IOStream *stream){
     int err = 1;
     if(stream->methods && stream->methods->close){
         err = stream->methods->close(stream);
+        stream->closed = 1;
     }
     return err;
 }
@@ -189,7 +196,7 @@ static inline int IOStream_close(IOStream *stream){
  * @return 1 if closed, 0 otherwise
  */
 static inline int IOStream_is_closed(IOStream *stream){
-  return stream->closed;
+    return stream->closed;
 }
 
 /** Free the memory used by the stream.
@@ -197,11 +204,14 @@ static inline int IOStream_is_closed(IOStream *stream){
  * @param stream to free
  */
 static inline void IOStream_free(IOStream *stream){
-  if(stream->methods && stream->methods->free){
-    stream->methods->free(stream);
-  }
-  *stream = (IOStream){};
-  deallocate(stream);
+    if(!stream->closed && stream->methods && stream->methods->close){
+        stream->methods->close(stream);
+    }
+    if(stream->methods && stream->methods->free){
+        stream->methods->free(stream);
+    }
+    *stream = (IOStream){};
+    deallocate(stream);
 }